Skip to content

1.0.4.6#40

Open
idenys wants to merge 4 commits intomasterfrom
1.0.4.6
Open

1.0.4.6#40
idenys wants to merge 4 commits intomasterfrom
1.0.4.6

Conversation

@idenys
Copy link
Copy Markdown
Owner

@idenys idenys commented Apr 8, 2026

This pull request introduces several enhancements and updates to the MXTires.Microdata library, focusing on extending the schema.org data model, improving property validation, modernizing project files, and updating dependencies. The most significant changes include the addition of new schema.org types and properties, refactoring for stricter property validation, and updates to project and test configurations for better compatibility and maintainability.

Schema.org Model Extensions:

  • Added a new class MemberProgramTier in Intangible/MemberProgramTier.cs to represent membership program tiers, including a TierRequirement property.
  • Extended PriceSpecification with new properties: EligibleQuantity, EligibleTransactionVolume, MembershipPointsEarned, and EligibleMemberTier, along with improved documentation and JSON serialization attributes. [1] [2]
  • Added support for the priceCurrency property in PriceSpecification with appropriate JSON serialization.

Property Validation and Serialization Improvements:

  • Refactored the Thing class to use static TypeValidator instances for stricter and more maintainable property type validation on fields like image, mainEntityOfPage, identifier, and subjectOf. Also, added support for disambiguatingDescription and improved JSON serialization settings to ignore null values on most properties. [1] [2]
  • Updated the PriceSpecification property in Offer.cs to use a more robust TypeValidator configuration for type checking.

Project and Dependency Updates:

  • Updated the main project to support .NET 8.0, removed support for .NET Core 3.1 and .NET 5.0, and incremented the version to 1.0.4.6. [1] [2]
  • Upgraded the Newtonsoft.Json dependency from 13.0.3 to 13.0.4.

Test Project Modernization:

  • Migrated the test project to the new SDK-style .csproj format, targeting .NET Framework 4.8, and switched to the latest MSTest NuGet packages.
  • Removed the obsolete AssemblyInfo.cs file from the test project.

Minor Cleanups:

  • Removed unused using directives from Products/Product.cs for code cleanliness.
  • Added missing using directives in PriceSpecification.cs and Thing.cs to support new features and maintain consistency. [1] [2]

These changes collectively improve the library's schema coverage, robustness, maintainability, and compatibility with modern .NET tooling.

Copilot AI review requested due to automatic review settings April 8, 2026 22:53
@idenys idenys self-assigned this Apr 8, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR bumps MXTires.Microdata to 1.0.4.6 and expands the schema.org model and validation/serialization behavior, while modernizing project/test configuration and updating dependencies.

Changes:

  • Extend the schema model with MemberProgramTier and new PriceSpecification properties.
  • Refactor Thing for stricter type validation, add new schema.org properties, and centralize JSON serialization settings.
  • Modernize the test project to SDK-style and update library target frameworks/dependencies (incl. .NET 8 and Newtonsoft.Json 13.0.4).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
Thing.cs Adds identifier/subjectOf/disambiguatingDescription, centralizes JSON serialization, and refactors type validation.
PriceSpecification.cs Adds schema.org properties like eligibleQuantity, eligibleTransactionVolume, membershipPointsEarned, eligibleMemberTier, and priceCurrency.
Intangible/Offer.cs Updates priceSpecification validation to allow types from the PriceSpecifications namespace.
Intangible/MemberProgramTier.cs Introduces new schema type MemberProgramTier with tierRequirement.
MXTires.Microdata.csproj Updates version/framework targets and bumps Newtonsoft.Json dependency.
Tests/MXTires.Microdata.Tests.csproj Migrates tests to SDK-style net48 with MSTest NuGet packages.
Tests/Properties/AssemblyInfo.cs Removes legacy AssemblyInfo (consistent with SDK-style projects).
Products/Product.cs Removes unused using directives.
.gitignore Ignores .vs/ directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

new TypeValidator("MXTires.Microdata.CreativeWorks", null, new List<Type> { typeof(string), typeof(WebSite) });

private static readonly TypeValidator IdentifierValidator =
new TypeValidator("MXTires.Microdata.Intangible", null, new List<Type> { typeof(string), typeof(PropertyValue) });
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IdentifierValidator is constructed with the namespace "MXTires.Microdata.Intangible", which makes the identifier property accept any type in that namespace (e.g., Offer, Brand, etc.). Schema.org identifier should be limited to PropertyValue/Text/URL (and optionally collections of those), so this validator is overly permissive and defeats type checking. Prefer an explicit acceptable-type list (e.g., string/PropertyValue/(List/IList of those), and/or Uri) rather than a namespace-based validator here.

Suggested change
new TypeValidator("MXTires.Microdata.Intangible", null, new List<Type> { typeof(string), typeof(PropertyValue) });
new TypeValidator(new List<Type>
{
typeof(string),
typeof(PropertyValue),
typeof(Uri),
typeof(string[]),
typeof(PropertyValue[]),
typeof(Uri[]),
typeof(List<string>),
typeof(List<PropertyValue>),
typeof(List<Uri>),
typeof(IList<string>),
typeof(IList<PropertyValue>),
typeof(IList<Uri>)
});

Copilot uses AI. Check for mistakes.
Comment on lines +118 to +131
/// <summary>
/// The identifier property represents any kind of identifier for any kind of Thing, such as ISBNs, GTIN codes, UUIDs etc.
/// Schema.org: PropertyValue | Text | URL
/// </summary>
[JsonProperty("image")]
[JsonProperty("identifier", NullValueHandling = NullValueHandling.Ignore)]
public object Identifier
{
get { return identifier; }
set
{
IdentifierValidator.Validate(value);
identifier = value;
}
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding Thing.Identifier introduces a duplicate identifier JSON property for types that already declare their own Identifier (e.g., Intangible/JobPosting.cs has [JsonProperty("identifier")] public object Identifier). Newtonsoft.Json typically treats duplicate JSON property names in an inheritance chain as an error or will serialize unpredictably. Consider removing the duplicate derived property (or making the base property virtual and overriding it) so each type only contributes one identifier member.

Copilot uses AI. Check for mistakes.
get { return subjectOf; }
set
{
SubjectOfValidator.Validate(value);
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SubjectOf is documented as CreativeWork | Event, but SubjectOfValidator only allows types from MXTires.Microdata.CreativeWorks (and CreativeWork). This will throw when a consumer sets SubjectOf to MXTires.Microdata.Event. Add typeof(Event) (or otherwise include Event) to the validator so it matches the documented/schema.org union.

Suggested change
SubjectOfValidator.Validate(value);
if (!(value is Event))
{
SubjectOfValidator.Validate(value);
}

Copilot uses AI. Check for mistakes.
}

//#endregion
return "<script type=\"application/ld+json\">" + json + "</script>";
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When wrapping JSON-LD in a <script> tag, the raw JSON is concatenated without escaping. If any string property contains </script> (or similar), it can break out of the tag and create an XSS vector in HTML contexts. Consider escaping the closing script sequence (e.g., replacing </ with <\/) when wrapInScriptTag is true, or returning the script tag via an HTML-encoding helper.

Suggested change
return "<script type=\"application/ld+json\">" + json + "</script>";
var scriptSafeJson = json.Replace("</", "<\\/");
return "<script type=\"application/ld+json\">" + scriptSafeJson + "</script>";

Copilot uses AI. Check for mistakes.
var validator = new TypeValidator(
"MXTires.Microdata.Intangible.StructuredValues.PriceSpecifications",
null,
new List<Type>() { typeof(PriceSpecification), typeof(IList<PriceSpecification>) });
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeValidator checks exact runtime types; allowing typeof(IList<PriceSpecification>) does not allow a List<PriceSpecification> instance. As written, assigning a List<PriceSpecification> to Offer.PriceSpecification will fail validation. Add typeof(List<PriceSpecification>) (and/or update the validator strategy to use assignability) so common list implementations are accepted.

Suggested change
new List<Type>() { typeof(PriceSpecification), typeof(IList<PriceSpecification>) });
new List<Type>() { typeof(PriceSpecification), typeof(IList<PriceSpecification>), typeof(List<PriceSpecification>) });

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +90
/// <summary>
/// Number or QuantitativeValue - The number of membership points earned by the member.
/// If necessary, the unitText can be used to express the units the points are issued in. (E.g. stars, miles, etc.)
/// </summary>
/// <value>The membership points earned.</value>
[JsonProperty("membershipPointsEarned")]
public QuantitativeValue MembershipPointsEarned { get; set; }

Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

membershipPointsEarned is documented as Number or QuantitativeValue, but the property type is QuantitativeValue only. Elsewhere in the codebase, similar unions are modeled as object (e.g., FinancialProduct.AnnualPercentageRate). Consider changing this to object with type validation (number types + QuantitativeValue) so numeric values are supported.

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +4
<TargetFrameworks>net461;net472;net48;net6.0;net8.0</TargetFrameworks>
<Version>1.0.4.6</Version>
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TargetFrameworks no longer includes netcoreapp3.1, but the project file still contains an ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'" later in the file. This condition is now dead code and can be removed to avoid confusion and accidental maintenance drift.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants